1352497 - Remove about:healthreport

Status: RESOLVED FIXED

(Toolkit :: Telemetry, enhancement, P1)

Description

[mheubusch]

Attachment: Screen Shot of Hamburger Menu>?>Help Menu

Please remove the access points to the about:healthreport page.  On all platforms from the HamburgerMenu>? button>Help menu and on Mac also from the top Help menu.

Comment 1

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

In the Preferences/Options,
new preferences (Nightly; after bug 1335907 landed): Updates > Enable Health Report
old preferences: Advanced > Data Choices > Enable Health Report
has as description: "Helps you understand your browser performance and shares data with Mozilla about your browser health"

Should this be shortened to "Shares data with Mozilla about your browser health"?

Furthermore, Telemetry depends on FHR on desktop in the preferences, but not on Android.

Comment 2

[Georg Fritzsche [:gfritzsche]]

(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #1)
> In the Preferences/Options,
> new preferences (Nightly; after bug 1335907 landed): Updates > Enable Health
> Report
> old preferences: Advanced > Data Choices > Enable Health Report
> has as description: "Helps you understand your browser performance and
> shares data with Mozilla about your browser health"
> 
> Should this be shortened to "Shares data with Mozilla about your browser
> health"?

See bug 1348074 on FHR/Telemetry description concerns.
This bug is separate and only about not exposing about:healthreport directly anymore.

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Bug 1352497 - Remove link to about:healthreport from menus: browser/.

Opening about:healthreport by using the locationbar will still work.

Review commit: https://reviewboard.mozilla.org/r/125964/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/125964/

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Bug 1352497 - Remove link to about:healthreport from menus: android.

Review commit: https://reviewboard.mozilla.org/r/125966/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/125966/

Comment 5

[Benjamin Smedberg]

Has this gone through a legal review with Marshall? I fully support this but there have been concerns before.

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8853948 [details]
Bug 1352497 - Remove link to about:healthreport from menus: android.

https://reviewboard.mozilla.org/r/125966/#review129358

LGTM

Comment 7

[Dão Gottwald [:dao]]

So these patches remove the references to about:healthreport but the page still exists, basically as dead code at that point. What's the longer-term plan for about:healthreport?

Comment 8

[Benjamin Smedberg]

Please DO NOT land this without confirmation from Michelle that this has gone through risk analysis.

If we can remove this, we can probably remove the about:healthreport mechanism altogether.

Comment 9

[mheubusch]

(In reply to Benjamin Smedberg [:bsmedberg] from comment #5)
> Has this gone through a legal review with Marshall? I fully support this but
> there have been concerns before.

Hello Benjamin - I filed this bug per a discussion with Mika. Since we are only removing the access points from the UI but not the page itself, she simply asked that I notify Marshall so that we had a response ready in case anyone thought the page itself was gone.  He is cc'ed on the bug. I will file a separate bug and x-ref it here just to be thorough.

Comment 10

[Benjamin Smedberg]

There is absolutely no point in keeping the about:healthreport page if it's not in the UI.

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 8853947 [details]
Bug 1352497 - Remove link to about:healthreport from menus: browser/.

I haven't heard back from Mika, but I think general consensus was that we should completely remove this page.

Comment 12

[:Atoll]

Hi all. I'm visiting from bug 1346273 where we're trying, and failing, to apply modern web security principles to about:healthreport. Can we get this removal shipped in Firefox soon?

Comment 13

[April King [:April]]

Looking at what about:healthreport does, it almost seems like this would be better served as a wrapped webextension, if we want it to continue to exist.

Comment 14

[Dão Gottwald [:dao]]

Who can make a final decision here?

Comment 15

[Benjamin Smedberg]

pdol owns this decision.

Comment 16

[Peter Dolanjski [:pdol]]

I spoke with Elvin about this (since Mika is now out on leave).
We will likely want to move forward with removal in v56, but Elvin is trying to coordinate some governance communication about a number of changes for telemetry (preferences) and healthreport. (that we may want to put out before the removal)
He asked that we hold off until he comes back with confirmation to proceed. I'm NI'ing him here so he can reply directly.

Comment 17

[Elvin Lee [:ellee] (no longer with Mozilla)]

Hi, confirming that we can move forward with the work to remove for 56+. How are we planning on informing users in-product what happened to FHR?

Comment 18

[Peter Dolanjski [:pdol]]

(In reply to Elvin Lee [:ellee] (use NEEDINFO please) from comment #17)
> Hi, confirming that we can move forward with the work to remove for 56+. How
> are we planning on informing users in-product what happened to FHR?

What happened to FHR in terms of it just becoming telemetry or do you mean about:healthreport, specifically?

Comment 19

[Elvin Lee [:ellee] (no longer with Mozilla)]

What happened to about:healthreport, specifically. The concern being that few users who do go to about:healthreport will suddenly get "address not valid"

Comment 20

[Dão Gottwald [:dao]]

(In reply to Elvin Lee [:ellee] (use NEEDINFO please) from comment #19)
> What happened to about:healthreport, specifically. The concern being that
> few users who do go to about:healthreport will suddenly get "address not
> valid"

I think the standard way to do this is to put in the release notes.

We could also have a stub about:healthreport page explaining that the feature doesn't exist anymore. However, I suspect for most people who ever saw or used this feature, the Help menu has been the entry point, so once that menu item is gone, leaving about:healthreport behind, even as a stub, seems pointless.

Comment 21

[Peter Dolanjski [:pdol]]

Yes, release notes.  If someone goes there and it's not there, they can check the release notes.  That's probably the best we can do and maybe add something in SUMO.

Elvin, does that work for you?

Joni, can you help with the SUMO piece?  If we have any information on about:health report, it should be updated to say that the page has been removed.

Comment 22

[Elvin Lee [:ellee] (no longer with Mozilla)]

That's fine - probably should indicate the version it is being removed for in the SUMO article too.

Comment 23

[Dão Gottwald [:dao]]

Sebastian, are you available to pick this up again?

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

(In reply to Dão Gottwald [::dao] from comment #23)
> Sebastian, are you available to pick this up again?
Yes, will do.

Comment 25

[John Gruen [:jgruen]]

Just curious, are the underlying about:config settings being stripped out as well?

Comment 26

[Dão Gottwald [:dao]]

(In reply to [:jgruen] from comment #25)
> Just curious, are the underlying about:config settings being stripped out as
> well?

What prefs in particular?

Any pref that's needed only for the about:healthreport page should ideally go away here. Other prefs would be off-topic as far as this bug is concerned.

Comment 27

[John Gruen [:jgruen]]

Screenshots uses `datareporting.healthreport.uploadEnabled` to determine whether or not we collect data. I've already filed an issue to move us off this pref. https://github.com/mozilla-services/screenshots/issues/3390

Comment 28

[Dão Gottwald [:dao]]

(In reply to [:jgruen] from comment #27)
> `datareporting.healthreport.uploadEnabled`

I don't think this has anything to do with this bug.

Comment 29

[John Gruen [:jgruen]]

Fair enough, we're going to make the switch anyway since FHR seems to be going into the dustbin.

Comment 30

[Georg Fritzsche [:gfritzsche]]

(In reply to [:jgruen] from comment #27)
> Screenshots uses `datareporting.healthreport.uploadEnabled` to determine
> whether or not we collect data. I've already filed an issue to move us off
> this pref. https://github.com/mozilla-services/screenshots/issues/3390

This pref still controls all data upload, even with Telemetry. Let's check what exactly can get cleanup.
To simplify things we could limit this bug to just remove what is clearly only used for the about:healthreport page and move anything that requires investigation or discussion to a follow-up.

I'll follow up on the GH issue.

Comment 31

[[:philipp]]

please help us what to do with the existing sumo article at https://support.mozilla.org/kb/firefox-health-report-understand-your-browser-perf documenting fhr (bug 1400272) :-)
we can add version specific content there - in which version of firefox are you planning to totally remove the page?

Comment 32

[Dão Gottwald [:dao]]

At this point this will miss 57 unless we still uplift it. Sebastian, are you working on this or should somebody else take over?

Comment 33

[Dão Gottwald [:dao]]

Attachment: Bug 1352497 - Remove about:healthreport.

Review commit: https://reviewboard.mozilla.org/r/200278/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/200278/

Comment 34

[Dão Gottwald [:dao]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=39d4f66d742c60d4e6af1537b1bba4f37453c9d2

Comment 35

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8928932 [details]
Bug 1352497 - Remove about:healthreport.

https://reviewboard.mozilla.org/r/200278/#review206456

Cheers for this, i'm happy to see this go!

One thing: far as i can tell, the selfsupport service is only used for about:healthreport and can go too:
https://searchfox.org/mozilla-central/search?q=MozSelfSupport&path=
https://searchfox.org/mozilla-central/search?q=SelfSupport&case=false&regexp=false&path=

Also, i think we can remove more preferences (see below).

::: testing/geckodriver/src/prefs.rs:121
(Diff revision 1)
>          ("browser.warnOnQuit", Pref::new(false)),
>  
>          // Do not show datareporting policy notifications which can
>          // interfere with tests
> -        ("datareporting.healthreport.about.reportUrl", Pref::new("http://%(server)s/dummy/abouthealthreport/")),
>          ("datareporting.healthreport.documentServerURI", Pref::new("http://%(server)s/dummy/healthreport/")),

I think we can remove all `datareporting.*` prefs, except:
- `datareporting.policy.dataSubmission*`
- `datareporting.healthreport.uploadEnabled`
- `datareporting.healthreport.infoURL`

We should double-check though for what Fennec uses.

::: toolkit/components/telemetry/docs/fhr/architecture.rst
(Diff revision 1)
>  service.loadDelayMsec
>     How long (in milliseconds) after initial application start should FHR
>     wait before initializing.
>  
>     FHR may initialize sooner than this if the FHR service is requested.
> -   This will happen if e.g. the user goes to ``about:healthreport``.

FWIW, these docs are marked as obsolete already:
https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/fhr/index.html

We already removed the FHR service before and only keep these docs for data archeology purposes.

::: toolkit/components/telemetry/docs/internals/preferences.rst:31
(Diff revision 1)
>  
>  ``MOZ_SERVICES_HEALTHREPORT``
>  
>    When Defined (which it is on most platforms):
>  
> -  * Builds ``about:healthreport`` and associated underpinnings (see `healthreport <../fhr/index>`)
> +  * Builds healthreport underpinnings (see `healthreport <../fhr/index>`)

While we're here, could you also remove this bullet?
We already removed FHR a while ago.

Comment 36

[Georg Fritzsche [:gfritzsche]]

Sebastian, do you know who can give high-level feedback on the Android removals here?

Comment 37

[Dão Gottwald [:dao]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #35)
> One thing: far as i can tell, the selfsupport service is only used for
> about:healthreport and can go too:
> https://searchfox.org/mozilla-central/search?q=MozSelfSupport&path=
> https://searchfox.org/mozilla-central/
> search?q=SelfSupport&case=false&regexp=false&path=

It's a web API, right? I can't tell from those searches where it's used. New bug please.

> ::: testing/geckodriver/src/prefs.rs:121
> (Diff revision 1)
> >          ("browser.warnOnQuit", Pref::new(false)),
> >  
> >          // Do not show datareporting policy notifications which can
> >          // interfere with tests
> > -        ("datareporting.healthreport.about.reportUrl", Pref::new("http://%(server)s/dummy/abouthealthreport/")),
> >          ("datareporting.healthreport.documentServerURI", Pref::new("http://%(server)s/dummy/healthreport/")),
> 
> I think we can remove all `datareporting.*` prefs, except:
> - `datareporting.policy.dataSubmission*`
> - `datareporting.healthreport.uploadEnabled`
> - `datareporting.healthreport.infoURL`

Ditto, this should be a separate bug. Looks like some of these prefs are already unused today.

Comment 38

[Dão Gottwald [:dao]]

Comment on attachment 8928932 [details]
Bug 1352497 - Remove about:healthreport.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200278/diff/1-2/

Comment 39

[Georg Fritzsche [:gfritzsche]]

(In reply to Dão Gottwald [::dao] from comment #37)
> (In reply to Georg Fritzsche [:gfritzsche] from comment #35)
> > One thing: far as i can tell, the selfsupport service is only used for
> > about:healthreport and can go too:
> > https://searchfox.org/mozilla-central/search?q=MozSelfSupport&path=
> > https://searchfox.org/mozilla-central/
> > search?q=SelfSupport&case=false&regexp=false&path=
> 
> It's a web API, right? I can't tell from those searches where it's used. New
> bug please.

Sounds good.

> > ::: testing/geckodriver/src/prefs.rs:121
> > (Diff revision 1)
> > >          ("browser.warnOnQuit", Pref::new(false)),
> > >  
> > >          // Do not show datareporting policy notifications which can
> > >          // interfere with tests
> > > -        ("datareporting.healthreport.about.reportUrl", Pref::new("http://%(server)s/dummy/abouthealthreport/")),
> > >          ("datareporting.healthreport.documentServerURI", Pref::new("http://%(server)s/dummy/healthreport/")),
> > 
> > I think we can remove all `datareporting.*` prefs, except:
> > - `datareporting.policy.dataSubmission*`
> > - `datareporting.healthreport.uploadEnabled`
> > - `datareporting.healthreport.infoURL`
> 
> Ditto, this should be a separate bug. Looks like some of these prefs are
> already unused today.

Right. So, if i understand this correctly, you are just removing the one specific pref related to the about:healthreport content?

Comment 40

[Dão Gottwald [:dao]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #39)
> Right. So, if i understand this correctly, you are just removing the one
> specific pref related to the about:healthreport content?

Yeah.

Comment 41

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8928932 [details]
Bug 1352497 - Remove about:healthreport.

https://reviewboard.mozilla.org/r/200278/#review206970

This looks good to me overall.
Two things though:

1) I'd really want a mobile peer to look over it if the changes make sense to them.

2) Removing `mobile/android/chrome/content/healthreport-prefs.js` might have side-effects as it includes `datareporting.healthreport.uploadEnabled`.
I'm not off-hand 100% sure that this won't effect Fennec Telemetry in some way.
I'd prefer to leave the pref `uploadEnabled` in for Fennec and let us clean this up when we get to sorting the mobile Telemetry situation.
Alternatively we can trace the pref usage through the tree and verify, but that will take a bit.

Comment 42

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8928932 [details]
Bug 1352497 - Remove about:healthreport.

https://reviewboard.mozilla.org/r/200278/#review206976

Comment 43

[Dão Gottwald [:dao]]

Comment on attachment 8928932 [details]
Bug 1352497 - Remove about:healthreport.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200278/diff/2-3/

Comment 44

[Dão Gottwald [:dao]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4878506603f292ed1385d98b0d917b56a40640f

Comment 45

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8928932 [details]
Bug 1352497 - Remove about:healthreport.

https://reviewboard.mozilla.org/r/200278/#review208022

The code looks goot to me. I've pulled it and tested in my local. In Fennec side it works fine. But I still don't know the reason to remove this. It'll be great if you can give us more background about this.

Comment 46

[Dão Gottwald [:dao]]

(In reply to Nevin Chen [:nechen] from comment #45)
> Comment on attachment 8928932 [details]
> Bug 1352497 - Remove about:healthreport.
> 
> https://reviewboard.mozilla.org/r/200278/#review208022
> 
> The code looks goot to me. I've pulled it and tested in my local. In Fennec
> side it works fine. But I still don't know the reason to remove this. It'll
> be great if you can give us more background about this.

The feature is unneeded and nobody wants to maintain it.

Comment 47

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be86ccde4f4a
Remove about:healthreport. r=gfritzsche,nechen

Comment 48

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/be86ccde4f4a

Comment 49

[Cosmin Sabou [:CosminS]]

Backed out for bc failures on Windows 7 debug with e10s at docshell/test/browser/browser_bug1309900_crossProcessHistoryNavigation.js

https://treeherder.mozilla.org/logviewer.html#?job_id=147537473&repo=mozilla-central&lineNumber=13901

https://hg.mozilla.org/mozilla-central/rev/3e14872b31a7b1b207605d09b78fbaaf21f1bba7

https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=f6f7da6085aa13abb6dce67f7eb21cdc000fd4fb&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&selectedJob=147537473

Also would you please look at these tier 2 Linux x64 JSDCov opt failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-searchStr=ba2e181e09bd32af6e2b059a56058a9b498da49c&selectedJob=147639971 

https://treeherder.mozilla.org/logviewer.html#?job_id=147639971&repo=mozilla-central&lineNumber=4320

Comment 50

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

s/with e10s/without e10s/

Comment 51

[Jim Jeffery not reading bug-mail 1/2/11]

Reopened due to back-out

Comment 52

[Dão Gottwald [:dao]]

Comment on attachment 8928932 [details]
Bug 1352497 - Remove about:healthreport.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/200278/diff/3-4/

Comment 53

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a1a46675739
Remove about:healthreport. r=gfritzsche,nechen

Comment 54

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/7a1a46675739

Comment 55

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7a1a46675739

Comment 56

[Pascal Chevrel:pascalc]

(In reply to Dão Gottwald [::dao] from comment #20)
> (In reply to Elvin Lee [:ellee] (use NEEDINFO please) from comment #19)
> > What happened to about:healthreport, specifically. The concern being that
> > few users who do go to about:healthreport will suddenly get "address not
> > valid"
> 
> I think the standard way to do this is to put in the release notes.
> 

Do you still intend to have it mentionned in the release notes? If so, please nominate this bug for release note addition. Thanks

Comment 57

[Dão Gottwald [:dao]]

Release Note Request (optional, but appreciated)
[Why is this notable]: users may have used this page to understand what data Firefox sends to Mozilla
[Affects Firefox for Android]: yes
[Suggested wording]: about:healthreport (Firefox Health Report) has been removed in favor of about:telemetry
[Links (documentation, blog post, etc)]:

Comment 58

[Liz Henry (:lizzard) (relman/hg->git project)]

Noted for 59.0b1 as: Changed the location of Firefox Health Report from about:healthreport to about:telemetry

Comment 59

[Andreas Tolfsen ❲:ato❳]

Note that this patch removed the
datareporting.healthreport.about.reportUrl pref from various automation
tools in the tree that targets release channels out-of-tree.
This change should not have been made without approval from a
peer of the Testing component.  I’m remedying this as part of
https://bugzilla.mozilla.org/show_bug.cgi?id=1401129.